Skip to content

Aligned normal_lccdf and std_normal_lccdf with the lcdf functions - #3363

Open
jaburgoyne wants to merge 1 commit into
stan-dev:developfrom
jaburgoyne:fix/normal_lccdf
Open

Aligned normal_lccdf and std_normal_lccdf with the lcdf functions#3363
jaburgoyne wants to merge 1 commit into
stan-dev:developfrom
jaburgoyne:fix/normal_lccdf

Conversation

@jaburgoyne

@jaburgoyne jaburgoyne commented Aug 21, 2026

Copy link
Copy Markdown

Summary

This pull request addresses #1985 by updating normal_lccdf and std_normal_lccdf to call normal_lcdf and std_normal_lcdf with the signs of x and mu reversed.

Tests

The tests have been updated to use the latest values of pnorm in R 4.6.1. These are more direct tests than the previous versions, which worked via an exp call, somewhat defeating the purpose of having separate lcdf and lccdf functions.

Side Effects

None.

Release notes

normal_lccdf and std_normal_lccdf updated to use normal_lcdf and std_normal_lcdf implementations

Checklist

  • Copyright holder: John Ashley Burgoyne

    The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
    - Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
    - Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

  • the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • the new changes are tested

@SteveBronder

Copy link
Copy Markdown
Collaborator

Fyi our backend CI is migrating to a new server cluster so the jenkins will hiccup for a minute while we get everything settled

@SteveBronder

Copy link
Copy Markdown
Collaborator

It looks like your changes exposed an issue in the opencl code. Let me look into this

@SteveBronder

SteveBronder commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

@jaburgoyne this is interesting, for the tests it looks like there may be an autodiff issue with using the symmetry trick here (which implies there is something wrong with our code that your code is showing us). At y=-50 we are getting an error for the hessian calculation vs finite difference.

./test/unit/math/expect_near_rel.hpp:90: Failure
Value of: stan::math::is_nan(x1) && stan::math::is_nan(x2)
  Actual: false
Expected: true
expect_near_rel(0, nan)
expect_near_rel; require items x1(0, 0) = x2(0, 0): hessian() Hessian for finite_diff vs fvar<var>

./test/unit/math/expect_near_rel.hpp:90: Failure
Value of: stan::math::is_nan(x1) && stan::math::is_nan(x2)
  Actual: false
Expected: true
expect_near_rel(0, nan)
expect_near_rel; require items x1(0, 0) = x2(0, 0): hessian_fvar() Hessian for finite_diff vs fvar<fvar<double>>

[  FAILED  ] AgradRev.mathMixScalFun_normal_lccdf (1 ms)

This passed at y=50 previously and your code changes test the negative boundary which is why the error shows up now. So this issue existed before your changes. I need to dive into this a bit more, would you mind taking a look as well? Claude says it has a solution that looks simple'ish and I'll make a little branch for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants